Tranche G: conversation attribution — report, gates, read-switch, write-deny - #1432

G1-a: Add broadcast bucket. Broadcasts with NULL conversation_id are
now a separate flip-blocking bucket ('unattributed — broadcast'). The
backfill service skips broadcasts (backfill.go:127), so they are never
attributed. All three read-switch sites (handlers_messages.go:70, :259,
handlers_chat_v2.go:1782) scope by ConversationID with no broadcast-
specific alternative path — broadcasts become invisible at the flip.

G1-b: Reconciliation check. On all-projects runs, compare the report's
unattributed total against CountUnbackfilledMessages(ctx, ""). Print a
loud RECONCILIATION MISMATCH line if they disagree, and treat the
mismatch as flip-blocking.

G1-c: Behavioral production derivation test. Table of KeyInputs with
known-pass and known-fail cases run through the classifier; asserts
exact bucket placement. Mutate/fail/revert/pass proven.

…aveat

G4-a (DEF-58): Add negative compile-time gate asserting brokerIdentityImpl
satisfies neither UserIdentity nor AgentIdentity. Document the intentional
empty-SenderID comparison in the broker delivery path.

G4-b (DEF-79): Add PathRecorder context-keyed trace mechanism and instrument
handleAgentMessage with 12 ordered checkpoints. TestDEF79_ProductionPathTrace
sends a user→agent message through the full HTTP handler and asserts the
step sequence; removing any step from the production code fails the test.

G4-c (DEF-80): Add unbackfilled_blind_spot caveat to the divergence board
explicitly stating that messages with empty ConversationID (pre-dual-write)
are invisible to the consistency check and a clean board is not evidence of
clean data.

…veat

G4-d: List every untraced path (agent-scoped route, agent→agent,
broker-inbound, group fan-out, managed-agent, outbound) in a comment
next to expectedPathSteps so a green result is not mistaken for full
path coverage.

G4-e: Add sampling_window caveat to the divergence board disclosing
the 50/25-row lookup limits in CheckConversationConsistency — the
mismatch count is a lower bound on a sample, not a census.

Remove the channel+thread fallback at all three read sites when the
ConversationReadSwitch is ON. Unresolved conversations now return HTTP
409 with machine-readable code "conversation_not_resolved" instead of
silently falling back to the legacy filter.

Fix the DM key parse bug: a key with a part count other than 5 now
returns 409 with code "invalid_dm_key" (defense-in-depth; validDMKey's
regex already rejects non-5-part keys at the HTTP layer with 400).

Sites modified:
- S1: handleConversationHistory (handlers_chat_v2.go)
- S2: handleMessages (handlers_messages.go)
- S3: handleAgentMessages (handlers_messages.go)

Switch-OFF behaviour is unchanged. Agent lookup failures at S2 still
skip the conversation path per R-9 discipline.

AC-G3-1 through AC-G3-5 covered by tests in handlers_read_switch_test.go.

…r (G3-d/e)

G3-d: Carry Channel:"web" into the switch-on filter at
handleConversationHistory. Without this, flipping the switch silently
drops the channel constraint and widens visibility to messages from
non-web surfaces (discord, telegram) sharing the same conversation_id.
Widening is the direction we cannot take back. Test proves discord
messages in the same conversation are excluded.

G3-e: Add SwitchBypassCounter (messaging.SwitchBypassMetrics) measuring
switch coverage separately from migration readiness (DivergenceMetrics).
Five reason labels, each with a test:
  - slug_param: S2, agent param is a slug (uuid.Parse fails)
  - agent_not_found: S2, agent UUID not in store
  - non_dm_key: S2, no agent param → no DM key to derive
  - wcs_nil: S1, webChatStore nil for non-DM key (early return)
  - non_web_channel: S3, channel not web/"" and no thread_id

When threadID is present but agent.ProjectID is empty (or nil UUID),
the handler previously fell through to the DM branch — silently serving
DM-scoped results for a thread query. This is a wrong-answer-with-200,
the exact failure class Tranche G exists to remove.

Make threadID the primary discriminator in S3's switch-on block:
- threadID present + no project → 409 with thread_project_required
- threadID present + has project → thread resolution (existing path)
- no threadID + web/empty channel → DM resolution (existing path)
- no threadID + non-web channel → bypass counter (existing path)

The check covers both empty string and uuid.Nil.String() since the ent
store always returns a UUID string for ProjectID.

The bug was threadID!="" with empty ProjectID falling through to the DM
branch. The comment described the opposite, making it impossible for the
next reader to reconstruct why the guard exists.

Reverse the B10 contract on the write path: conversation key derivation
and resolution failures now deny the write (message not persisted, not
published) instead of logging and continuing.

Producer changes:
- ResolveOrCreateDMConversation: returns (*ConversationResult, error)
- ResolveOrCreateThreadConversation: returns (*ConversationResult, error)
- ResolveOrCreateConversationByKey: returns (*ConversationResult, error)
  All nil-return points now return (nil, error) with descriptive messages.

Consumer changes (21 swallow points flipped to denials):
- handlers_agent_messaging.go: derive/resolve errors → 400/500
- handlers_broker_inbound.go: resolve errors → 500
- handlers_chat_v2.go: resolve errors → 500, return ""
- messagebroker.go: resolve errors → log + return (no HTTP)
- notifications.go: resolve errors → log + return

Two deliberate exceptions preserved:
- EnsureParticipant failure stays non-fatal (listing concern, not access)
- Federated subscriber UUID parse failure still skips (not denied)

ValidateAttributed (previously unreachable behind nil guards) now runs
unconditionally on all three handler paths.

Test fixture updates:
- Non-UUID sender/recipient IDs replaced with tid() UUIDs
- Non-canonical DM keys replaced with DMConversationKey() derivation
- Topics given conversation_ids via setTopicConversationID helper

Acceptance tests added: AC-G2-1 through AC-G2-5.

…al counters

G2-f: All 25 write-path denial sites are now gated behind
ConversationWriteDenySwitch (defaults OFF). When OFF, behavior is
byte-for-byte identical to the base scion/tranche-g branch (B10
contract: log-and-continue). When ON, derivation/resolution failures
deny the write (G2 contract).

G2-g: Each denial site increments WriteDenialMetrics with a site label
(e.g. "chat_v2.human.thread", "mb.user.dm", "notif.inbox"). Counter
is in divergence.go, separate from DivergenceMetrics / SwitchBypassMetrics.

AC-G2-6: Integration test in handlers_chat_v2_test.go verifies switch-OFF
yields 201 and switch-ON yields 500 for the same topic without
conversation_id.

All HTTP-facing write-deny sites now return 409 Conflict with
machine-readable code "conversation_not_resolved" instead of 500
Internal Server Error. This matches G3's read-path shape: same failure,
same status, same code — so an operator who turns the switch ON can
distinguish "switch is correctly refusing unbackfilled topics" from
"something else broke".

409 is the semantically correct status: the request conflicts with the
current state of the resource (no conversation row), and retrying will
not help until backfill runs.

Sites changed:
- handlers_agent_messaging.go: outbound.derive, outbound.resolve,
  outbound.validate, agent_msg.phase11, agent_msg.derive,
  agent_msg.resolve, agent_msg.validate
- handlers_broker_inbound.go: broker.phase11, broker.thread,
  broker.dm, broker.validate
- handlers_chat_v2.go: chat_v2.agent_routed.thread,
  chat_v2.agent_routed.dm, chat_v2.agent_routed.validate,
  chat_v2.human.thread, chat_v2.human.dm

AC-G2-6 integration test updated to assert 409 + code
"conversation_not_resolved".

…ase)

…re-existing DBs

The CREATE UNIQUE INDEX on webchat_topic(conversation_id) in the DDL
block fails on databases where the table was created before #1380
added the column — CREATE TABLE IF NOT EXISTS is a no-op, so the
column is absent and the index creation errors. This prevents Init
from reaching the migration that would have added the column.

Remove the redundant index from the DDL in both SQLite and Postgres
store implementations. The addTopicConversationID migration already
creates both the column and the index, gated by migrationCompleted().

Fixes the "failed to initialize webchat store" warning that silently
disables the entire web chat surface on pre-existing deployments.

Add GET/PUT /api/v1/admin/messaging endpoints for the two Tranche G
operational switches (conversation_read_switch, conversation_write_deny_switch).

Follows the maintenance/project_defaults pattern: DB-only section with
KoanfPaths nil (not seeded at startup), presence-aware partial update
(PUT one switch without clearing the other), audit fields (updated_by,
origin:"managed", revision increment), and admin-gated via route guard.

Absent/empty/malformed DB rows continue to yield OFF on both switches
(fail-safe default preserved). No reader logic changed.

…-soft on error

The initOperationalSettings caller was gated on
strings.EqualFold(cfg.Database.Driver, "postgres"), but every callee is
already driver-agnostic (SQLite no-ops the advisory lock via the
AdvisoryLocker branch).  The gate made GetOperationalSettings() nil on
SQLite, so the messaging admin API returned 501 and both switches were
unreachable.

Changes:
1. Remove the postgres-only guard — initOperationalSettings now runs on
   all drivers.
2. On init failure, log at ERROR with the wrapped error and continue
   booting (fail-soft) rather than aborting the hub.  The messaging
   switches remain fail-closed (OFF) when OperationalSettings is nil.
3. Update doc comments on OperationalSettings and
   startSettingsPropagation to reflect driver-agnostic usage.
4. Add explicit fail-closed unit tests for both messaging switches
   covering all four degenerate inputs: absent row, empty {}, malformed
   JSON, and nil OperationalSettings pointer.

…ts (DEF-89)

handlers_chat_v2.go never populates ConversationID, leaving CreateTopic's
tested dual-write branch unreachable. New webchat topics were created with
no conversation row, causing the backfill to be a decaying artifact.

When ConversationID is empty and hasConversationsTable() returns true,
CreateTopic now generates a ConversationID before entering the existing
dual-write branch. The external_ref derivation (empty string) matches
backfillTopicConversations. hasConversationsTable() is called before
BeginTx, preserving INVARIANT U-TX-1 (same pattern as EnsureGeneralTopic).

Tests updated: backfill tests use raw SQL inserts to simulate pre-existing
data without conversation_id; new AutoGen tests cover the happy path and
MaxOpenConns=1 deadlock safety; all dual-write tests use context timeouts.

The previous commit fixed sqliteWebChatStore.CreateTopic; this extends
the same fix to pgWebChatStore.CreateTopic, which had the identical bug:
empty ConversationID took the legacy no-linkage INSERT path.

Unlike the SQLite store, generation is unconditional — Postgres Init()
migrations guarantee the conversations table exists, so there is no
hasConversationsTable() gate. The legacy path is removed (dead code);
the SQLite store retains it because hasConversationsTable() can return
false in pre-migration environments.

Behavior changes on Postgres (both tightenings):
- Empty ConversationID + empty ProjectID: previously succeeded silently
  via the legacy path (inserting a topic with no project_id). Now fails
  with "project_id is required" because the auto-generated ConversationID
  triggers the existing ProjectID guard. The handler always provides
  ProjectID.
- Partially-migrated DB without conversations table: previously succeeded
  via the legacy path (topic with no conversation). Now fails because the
  generated ConversationID triggers the dual-write tx, and the INSERT
  INTO conversations fails. Silent success becomes loud failure. This is
  intentional — a topic without a conversation is the defect being fixed.

NOTE: the Postgres path is unverified by test. No pg test infrastructure
exists in this repo (no testcontainers, dockertest, sqlmock, or postgres
build tag; zero test files reference pgWebChatStore). The SQLite store's
identical logic IS tested. Filed as DEF-99.

…-100)

ResolveThreadConversationForRead now accepts an optional
WithReadTopicLookup option that mirrors the write-path intercept in
ResolveOrCreateConversationByKey. For native web topics whose
conversations row has external_ref='', the resolver finds the
conversation_id via the webchat_topic row instead of the impossible
external_ref match that produced 409 on every read.

Both handler call sites updated:
- handlers_chat_v2.go S1: passes wcs (already holding the topic)
- handlers_messages.go S3: passes s.webChatStore (had no topic lookup)

Resolution order: topic lookup first; if store.ErrNotFound (not a native
topic), fall through to external_ref. Infrastructure errors do NOT fall
through. Non-native surfaces with well-formed external_ref are preserved.
DM keys are unaffected (kind="direct" bypasses the intercept).

Add TestFormatForDelivery_ByteIdentity asserting that FormatForDelivery
output is byte-identical to pinned golden strings for a representative
spread of inputs: plain, raw, urgent, broadcast, threaded, with
attachments, with metadata, group-set with recipients, system message,
metadata filtering, and a fully-featured combined message.

This is the Phase 9a safety net. Phase 9b introduces a new delivery
envelope — the byte-identity guard ensures the switch-off path remains
unchanged. Later phases must not weaken this test.

Pure test addition; no production code changes.

…t Refresh (DEF-92)

Today three getters — ConversationReadSwitch, ConversationWriteDenySwitch,
and ProjectDefaultScratchpad — silently swallow JSON parse failures,
returning the compiled default without logging. This makes a corrupted
hub_settings row invisible to operators.

Add a Malformed bool to sectionState, set it at parse time in Refresh
and Update via json.Valid, and log the failure at error level ONCE per
ingest. Getters can now distinguish "validated document" from
"unreadable document" — which is the prerequisite for making
default-ON and fail-closed simultaneously expressible (Phase 9a §4.6.2).

This commit changes no default values and no getter behaviour. The
Malformed field is generic to all sections, not just messaging, so
every section benefits from the parse-time detection.

…ON switch (Phase 9a)

Replace conversation_read_switch and conversation_write_deny_switch with
a single conversation_envelope_switch that:
  - defaults ON when the section is absent (compiled default)
  - defaults ON when the key is omitted from the document
  - returns OFF when the document is malformed (fail-closed, DEF-92)
  - returns the explicit value when the key is present

Key changes:

  opsettings/sections.go: add ConversationEnvelopeSwitch field; keep
    stale fields for backward-compatible deserialization only.

  opsettings/registry.go: update messaging schema to accept only
    conversation_envelope_switch. Stale keys are rejected on write
    (additionalProperties: false) and self-clean on first PUT.

  operational_settings.go: new ConversationEnvelopeSwitch() getter using
    the three-way sectionState split (absent → ON, malformed → OFF,
    omitted → ON). Old getters removed.

  admin_messaging.go: collapse from two fields to one. Explicit-null
    reset uses DeleteSection so the absent→default (ON) path runs,
    rather than writing a literal false (the f:=false trap in §4.6.3).

  server.go, handlers_messages.go, handlers_chat_v2.go: all callers
    updated to use ConversationEnvelopeSwitch.

No data migration. The new key is absent on every existing hub, so every
hub takes the compiled default (ON). Stale keys self-clean because
handlePutMessaging rebuilds the document from the Go struct.

Tests cover AC-9-7 (five stored states), AC-9-7a (stale-keys cutover),
AC-9-7b (malformed logged once per refresh), AC-9-7c (PUT cleans stale
keys), and AC-9-7d (null reset returns ON, not false).

…ence

Three fixes from review:

(1) DEF-92 type-mismatch observability. json.Valid catches syntactic
    malformation but not semantic — a document like
    {"conversation_envelope_switch":"yes"} is valid JSON but fails to
    unmarshal into *bool. Without detection, this falls through to the
    compiled default (ON), silently enabling the switch on a hub where
    an operator made a typo.

    Fix: at Refresh/Update time, after json.Valid passes, also
    unmarshal into the section's typed struct via the registry's New
    function. If that fails, set Malformed=true and log at ERROR with
    the unmarshal error. Generic to all sections.

(2) nil-ops GET divergence. handleGetMessaging returned ON when
    OperationalSettings was nil, but enforcement sites read
    `ops != nil && ops.ConversationEnvelopeSwitch()` → OFF. The admin
    endpoint was reporting a switch state that was not in effect.

    Fix: default to false (what enforcement does) when ops is nil.

(3) Count accuracy noted — no code change needed.

4 files changed: operational_settings.go (+24), operational_settings_test.go (+31),
admin_messaging.go (+4 -3), admin_messaging_test.go (+8 -10).

Add two tests proving the consolidated envelope switch goes live on
upgrade without data migration:

- TestPhase9a_UpgradeCutover_WriteDenyLiveByDefault: no messaging row
  present → absent-row compiled default (ON) → write-deny fires 409.

- TestPhase9a_UpgradeCutover_StaleKeysOnly_WriteDenyLive: messaging row
  with only the two stale keys (both false) → new key absent → compiled
  default (ON) → write-deny fires 409.

Also updates TestG2_AC6 comments to clarify the first half passes
because ops is nil (not because the switch is OFF).

…Phase 9b-i)

Add DeliveryText to StructuredMessage and MessageRequest so the hub can
pre-render the agent-facing envelope and the broker delivers it verbatim.

Broker preference order:
  1. req.DeliveryText (top-level wire field)
  2. req.StructuredMessage.DeliveryText (carrier on StructuredMessage)
  3. FormatForDelivery(req.StructuredMessage) (legacy fallback)
  4. req.Message (plain text fallback)

Co-located adapter (resolveDeliveryText) uses the same DeliveryText-first
preference. HTTP and control-channel transports promote DeliveryText to
the top-level wire field for the broker.

…elper (Phase 9b-ii)

Enrich ConversationResult with Kind, Surface, DisplayName from the
already-loaded store.Conversation at all resolution paths (zero extra
queries). Add RenderDeliveryText and RenderDeliveryTextWithLookup as the
single shared rendering entry point for all hub send paths.

Invariants enforced by the helper:
  - Never fabricates an identifier. msg.ID is set from the persisted row.
  - reply_to is always omitted (genuine reply targets arrive in 9c-iii).
  - When ConvResult is nil, conversation is absent (honest absence).

… agent message path

P-1: Add ConversationID to OutboundMessageRequest and propagate onto
structuredMsg so it survives through the broker path to deliverToUser.

P-2: Authorize caller-supplied ConversationID on the outbound handler.
Port the DEF-49 authorization block with direction-aware docstrings:
the agent is the SENDER (not recipient), so group-case asserts "the
conversation belongs to the sending agent's project." Parse failure
denies, always. Never normalise a DM key — a differing round-trip is
an error, not a rewrite.

P-3: deliverToUser honours a pre-resolved ConversationID from the
upstream handler instead of re-deriving. This is the edit that makes
explicit routing actually take effect — without it the handler's
resolution was discarded when the broker was present, producing the
inbound/outbound conversation split.

P-4: Open the CLI conv:<uuid> and #<thread> gates. Update help text
and rewrite SKILL.md so replying into the addressed conversation is
the described default behaviour, not an optional new capability.

AC-1 through AC-12 tests covering: round-trip per surface, unauthorised
assertion denied (403 + no row), no memory, proactive send unchanged,
absent field unchanged, metadata bypass blocked, SKILL.md content guard,
and missing-conversation mismatch signal.

…e double divergence log

BLOCKER 1: The pre-resolved ConversationID path in deliverToUser built a
minimal ConversationResult with empty ExternalRef, causing
ComputeDivergenceMatch to report a false routing-type-mismatch on every
correctly-routed explicit message. Fixed by adding a distinct
explicit-routing outcome (LogExplicitRouting) with its own counter that
bypasses ComputeDivergenceMatch entirely.

BLOCKER 2: AC-6 not met — LogDivergence fired twice per outbound reply
(once in the handler, once in deliverToUser). Removed the handler's
divergence block and consistency check; the broker's deliverToUser
handles both at persistence time. Guard expected count updated 7→6.

New test: TestDEF138_ExplicitRouting_NeverCountedAsMismatch exercises
deliverToUser with a pre-resolved ConversationID and asserts
DivergenceMetrics.Mismatches() does not increase. Mutation-verified:
replacing LogExplicitRouting body with DivergenceMetrics.Inc(false)
keeps the build green but fails the test.

Add explicit_routes counter to the divergence board response alongside
consistency_checks/consistency_mismatches. Add explicit_routing_adoption
caveat explaining that explicit routing is not a comparison — the caller
stated the conversation identity — and that the counter measures adoption
of the new routing path, not correctness.

explicit_routes / (comparisons + explicit_routes) approximates the
fraction of outbound traffic using explicit conversation routing.

An agent's reply was delivered to the right channel but persisted into a DM
instead of the thread it came from. All four surfaces set ThreadID inbound, so
the message resolved a group conversation; an agent's reply carries no ThreadID,
so the outbound path always took the DM branch.

The root cause was not the branch. An agent replies to a principal, and a
principal does not identify a conversation, so the system was answering a
question the caller never answered. The fix completes the address rather than
improving the guess.

Rule: explicit conversation wins; else explicit thread; else derive from the
caller's own supplied address; else error. No component consults a side-table
memory of where an agent was last active. The envelope already carries the
conversation, so a reply carries it back in-band and its absence genuinely
means 'not a reply' -- for which a derived DM is correct.

- OutboundMessageRequest gains conversation_id; the CLI conv:<uuid> and
  #<thread> gates are opened.
- Caller-asserted conversations are authorized before use, mirroring the DEF-49
  block but with docstrings rewritten for the sender direction -- in the
  sibling handler 'agent' is the recipient, so identical-looking code asserts a
  different claim.
- The handler's resolution now reaches the writer, collapsing a double
  resolution that previously discarded one result and logged divergence twice.
- Explicit routes bypass ComputeDivergenceMatch entirely via a separate
  LogExplicitRouting outcome and counter. Comparing is meaningless when the
  caller stated the answer; an earlier revision fed an empty ExternalRef into
  the comparison and reported a false routing-type-mismatch on every correctly
  routed message.
- explicit_routes is surfaced on the admin divergence board with a caveat
  stating it measures adoption, not correctness.
- SKILL.md makes replying into the addressed conversation the described
  default; it previously said conversation_id was 'not yet required'.

Reviewed over three rounds. Both blockers found in review were mutation-verified
by the architect against production-code mutations with the build green.

…ng channel surface

Conversations created via the thread path were always stamped surface="native"
regardless of the originating channel (discord, slack, teams, etc). The channel
was available in the request but not plumbed through to conversation creation.

Add WithThreadSurface option to ResolveOrCreateThreadConversation, mirroring
the existing WithSurface/WithTopicLookup pattern. Forward it to the shared
ResolveOrCreateConversationByKey sink. Update all three callers that handle
external-channel traffic:

- resolvePhase5Conversation (handlers_broker_inbound.go)
- deliverToUser thread branch (messagebroker.go)
- deliverToAgent thread branch (messagebroker.go)

handlers_chat_v2.go call sites are web-native (Channel:"web") and correctly
keep the "native" default — no change needed.

Empty channel is guarded: only non-empty values are forwarded, preserving the
"native" default per R3. First-writer-wins is inherent in the upsert design:
the (surface, external_ref) unique index means a new surface on the same
external_ref creates a separate conversation row, not an update.

…n test double

BLOCKER 1: All three call sites passed raw msg.Channel into WithThreadSurface
without validation. Channel "web" (used by handlers_chat_v2.go) is not a valid
surface enum value — passing it through would cause SurfaceValidator to reject
the conversation write, denying the message. This violates R3.

Add ChannelToSurface() as the single mapping function:
- Valid surface names (discord, slack, telegram, gchat, teams, native) pass
  through directly.
- "web" maps explicitly to "native" (web-chat is the native surface).
- Unknown/empty channels fall back to "native" with a WARN log.
- All three call sites now use ChannelToSurface() instead of raw channel.

BLOCKER 2: surfaceTrackingUpserter accepted any surface string, hiding defects
that production SurfaceValidator would reject. Now validates against the real
enum — rejects values the real store would reject.

New tests:
- ChannelToSurface: valid channels, "web"→"native", empty→"native",
  unknown→"native" with warning
- End-to-end: unknown channel resolves conversation successfully on "native",
  and raw unknown channel is rejected by the hardened test double

… nil-guard logger

Eliminate the three-copy drift hazard: validSurfaces (production whitelist),
validSurfaceEnum (test double), and the ent SurfaceValidator enum were linked
only by comments.

- Delete validSurfaceEnum entirely; the test double now uses validSurfaces.
- Add TestValidSurfaces_MatchesEntEnum: imports pkg/ent/conversation and
  asserts validSurfaces matches the SurfaceValidator enum in both directions.
  An addition or removal to either side fails the test.
- Guard ChannelToSurface against nil logger to prevent panic on exported API.

…ng cross-check

The prior cross-check hand-listed the six ent Surface constants. Adding a
seventh to the schema would not fail the test — the guard could not catch the
exact drift it existed to prevent.

Replace with a source-scanning test that reads the ent schema file
(pkg/ent/schema/conversation.go), extracts the Values(...) arguments for the
surface enum field via regex, and compares to validSurfaces in both directions.

Fail-closed: if the file cannot be read, the pattern cannot be matched, or
zero values are extracted, the test fails with a clear diagnostic.

Remove the pkg/ent/conversation import — no longer needed since the test reads
the schema source directly instead of importing generated constants.

…ing channel

Every conversation created through the thread path was stamped surface
'native', whatever channel produced it. Found by ptone on a live gteam test: a
Discord message returned an envelope reading surface: native.

The row was written wrong at creation, not rendered wrong. WithSurface was
reachable from two call sites, both gated on req.Surface and req.ExternalRef,
and no plugin sets either — so every inbound message took the thread branch,
which had no surface parameter at all. The hub already knew the channel; it
used it twenty lines away for the affinity record.

This matters more than a mislabelled field. conversation.go:89-92 declares a
unique index on the PAIR (surface, external_ref), so conversation identity is
per-channel by design — ptone's model, already in the schema. With surface
pinned to a constant the index contributed nothing and silently degraded to
uniqueness on external_ref alone. The fix reactivates a dormant constraint.

Consequence, deliberate and tested: an existing (native, thread:P:T) row and a
new (discord, thread:P:T) resolve to DIFFERENT conversations. Live histories
fork rather than merge. TestDEF140_DiscordThreadCreatesSeparateConversation
asserts the split happens and the old row is untouched.

- ChannelToSurface is the single mapping point: valid surfaces pass through,
  web maps to native explicitly, unknown falls back to native with a WARN.
  Review blocker: the first revision forwarded the raw channel, and 'web' is
  not in the enum, so it would have converted a working path into a denied
  write.
- The test double now rejects invalid enum values. Previously it accepted any
  string, which is why a change that could deny writes produced a green gate.
- TestValidSurfaces_MatchesSchemaEnum scans the ent schema declaration rather
  than a transcription of it, fail-closed on unreadable file, unmatched
  pattern or zero values. Two earlier attempts compared hand-written copies
  against each other and could not fail in the direction that mattered.

Architect verified independently: numstat, both mutation directions, and the
full gate.

…ation in routing provenance

Add ConversationAsserted to StructuredMessage and a three-way classification
switch in the broker so that explicit_routes counts caller assertions only,
derived_routes counts hub derivations, and the adoption ratio can go down.

P1: ConversationAsserted field on StructuredMessage; IncDerivedRouting,
    DerivedRoutes, LogDerivedRouting in pkg/messaging/divergence.go.
P2: asserted bool set true only after authorization in the handler's
    explicit branch; propagated at the convResult site.
P3: Broker classification switches on ConversationAsserted, not
    ConversationID != "". Honouring block unchanged (P-3 preserved).
P4: Admin board exposes derived_routes; caveat updated to state
    explicit_routes counts caller assertions only.
P5: Tests for AC-1 through AC-7. AC-4 mutations verified (3/3 caught,
    build green, semantic failure, clean restore).

Refs: DEF-141, DEFECTS.md [^81]

…ed binding from any JSON source

AC-5 required fix: the original json:"conversation_asserted,omitempty" tag
allowed three structs embedding StructuredMessage (inboundMessageRequest,
MessageRequest, BroadcastMessageRequest) to bind the field from request JSON.
Changed to json:"-" so the property holds by construction, not by reachability.

Added route-independent unmarshal test (TestDEF141_AC5_ConversationAsserted_UnmarshalIgnored)
that tests the tag directly — this test cannot rot regardless of future route changes.

Mutation verified: reverting json:"-" to json:"conversation_asserted,omitempty"
causes the unmarshal test to fail (build green, semantic failure).

…display names

resolveThread now collects ALL matches across all pages before resolving.
When >=2 group conversations in a project share a display_name, the send
is refused with reason:"ambiguous" and both candidates listed as
conv:<uuid> (surface=<surface>).

This addresses the DEF-140 fork path: (native, thread:P:T) and
(discord, thread:P:T) carry the same display_name. Pre-P1, the first
pagination hit was silently returned, delivering to a different audience
depending on sort order.

Tests (5 new):
  - TestResolve_Thread_Ambiguous_DEF140ForkPath: DEF-140 fixture
    (native + discord rows, same display_name) → ambiguous
  - TestResolve_Thread_Ambiguous_ErrorMessage: error format validation
  - TestResolve_Thread_SingleMatch_StillResolves: positive control
  - TestResolve_Thread_ThreeMatches_StillAmbiguous: 3-way ambiguity
  - TestResolve_Thread_Ambiguous_AcrossPages: cross-page detection (110 rows)

Mutation verified: reverting to first-match (return on first DisplayName hit)
causes 4 ambiguity tests to fail (build green, semantic failures).
SingleMatch positive control still passes under mutation.

…guity test

TestResolve_Thread_Ambiguous_AcrossPages used uuid.NewString() for fixture
IDs. mockStore sorts by ID for pagination, so random UUIDs gave random
sort order — both "target" rows landed on page 1 ~85% of the time, making
the test pass under a pagination-break mutation 17/20 runs.

Fix: use fmt.Sprintf("00000000-0000-0000-0000-%012d", i) so sorted order
equals insertion order and targets at i=5 and i=105 always straddle the
Limit:100 page boundary.

Mutation verified: breaking the pagination loop after page 1 now causes
the test to fail 20/20 (build green, semantic failure).

Known bounded gap: mockStore paginates on ID alone; production uses
keyset pagination on (created_at, id) via decodeCursor in
conversation_store.go. This test proves the loop iterates across pages;
it does not prove production pagination semantics.

…re through handler

P2: Added ConversationRef field to OutboundMessageRequest (hub) and its
hubclient mirror. Accepted forms: conv:<uuid>, @<agent-slug>, @<email>,
#<thread-name>. Mutually exclusive with ConversationID.

P3: Handler wiring in handleAgentOutboundMessage, in load-bearing order:
  1. Both ConversationRef and ConversationID set → 400 (before anything)
  2. Message validation BEFORE Resolve (Resolve can create a row)
  3. Resolve with ResolveContext built ONLY from the authenticated caller
     (agent.ProjectID, never from the request body)
  4. Resolved ID flows through the EXISTING DEF-138 authorization block
  5. asserted = true → explicit routing path

ELEVATED CONSTRAINT (G-1): ResolveContext.ProjectID is the containment
boundary for P1's ambiguity error disclosure. Every rctx field comes from
the authenticated caller. TestDEF142_G1_ResolveContext_ProjectFromAuth_NotBody
proves a foreign project's threads are invisible via conversation_ref.

Tests (8 new):
  - MutualExclusion_BothRefAndID: both fields → 400
  - ConversationRef_ConvUUID: conv:<uuid> resolves
  - ConversationRef_ThreadRef: #thread resolves
  - ConversationRef_NotFound: unknown thread → 400
  - ConversationRef_Ambiguous: DEF-140 fork path → 400 with candidates
  - ConversationRef_InvalidFormat: bad ref → 400
  - ConversationRef_SetsAsserted: explicit_routes increments (broker path)
  - G1_ResolveContext_ProjectFromAuth_NotBody: foreign project isolation

Mutation verified (AC-9): changing agent.ProjectID to req.ConversationID
in ResolveContext causes 6/8 tests to fail (build green, semantic failures).
G1 test specifically catches the project isolation breach.

…t resolve-or-create auth

AC-3: "not-found", "not-a-participant", and "boundary-violation" now
produce BYTE-IDENTICAL response bodies ("conversation_ref could not be
resolved"). The real reason is logged server-side only. Without this,
conv:<uuid> probes could distinguish "does not exist" from "exists but
sender is not a participant" — an enumeration oracle for conversation IDs.

"ambiguous" and "no-shared-project" remain distinct: ambiguity candidates
are group conversations in the caller's own project (already authorized),
and no-shared-project is a caller-side configuration error.

AC-3 mutation verified: un-collapsing (passing resErr.Error() through)
causes TestDEF142_AC3_NotFound_vs_NotParticipant_ByteIdentical to fail
with a diff showing "not found" vs "sender is not a participant" in the
response bodies. Build green, semantic failure.

AC-6: TestDEF142_AC6_ResolveOrCreate_FlowsThroughDEF138Auth verifies
that @agent-slug resolve-or-create (Created==true, Resolve skips its own
post-resolution auth) still flows through the DEF-138 authorization block
with asserted=true, causing explicit_routes to increment.

AC-6 mutation verified: removing the promotion (req.ConversationID =
resolveResult.ConversationID → _ = resolveResult) causes the test to fail
because explicit_routes stays at 0 — the resolved ID never enters the
DEF-138 block. Build green, semantic failure.

The AC-3 switch was a denylist: named reasons collapsed, default disclosed.
A future ResolutionError reason would land in default and disclose in full,
silently, with no test failing. That is fail-open on a disclosure control.

Inverted to allowlist: only "ambiguous" and "no-shared-project" disclose.
Default collapses. Same behaviour today, byte for byte. Different behaviour
on the day someone extends the enum: a new reason is collapsed until someone
deliberately adds it to the allowlist.

TestDEF142_AC3_FutureReason_CollapsedByDefault makes the allowlist
load-bearing via two mechanisms:
  1. Source scan: asserts the handler contains the allowlist case and NOT
     the denylist case (same pattern as consistency_check_guard_test.go)
  2. End-to-end: a known-collapsed reason produces the generic message
     without reason-specific text

Mutation verified: reverting to denylist shape causes the source scan to
fail. Build green, semantic failure.

…versation_ref

The CLI's two-step resolve-then-send pattern (call /conversations/resolve,
then send with the resolved ID) is replaced by passing conversation_ref
directly in the outbound message request. The server resolves it inline
(P3) and routes through the existing DEF-138 auth block.

Changes:
- Delete ConversationResolveRequest, ConversationResolveResponse types
  and ResolveConversation from pkg/hubclient/messages.go (interface +
  implementation)
- Delete /conversations/resolve mock from cmd/message_convref_test.go
- Rewrite sendMessageViaConversation in cmd/message.go:
  - Agent context (SCION_AGENT_NAME set): all ref kinds use outbound
    endpoint with conversation_ref
  - Human CLI context: @agent uses SendStructuredMessage; server derives
    the conversation from sender/recipient principals (DEF-138 Rule 3)
- Add TestSendMessageViaConversation_AgentRef_AgentContext test
- AC-8: grep finds zero occurrences of "conversations/resolve"

…ason

Extract the AC-3 disclosure decision into a pure function
disclosableResolutionReason(reason string) bool so the allowlist is
directly testable. The handler delegates to it; the switch moves inside.

Replace the source-grep test (Part 1) with a table-test that exercises
every known reason plus "some-future-reason" and "" — binding to the
actual artefact every reason passes through. Rename the end-to-end test
(Part 2) to TestDEF142_AC3_KnownReason_CollapsedEndToEnd since it tests
collapse of a known reason, not a future one.

Table-test cases:
  "ambiguous" → true, "no-shared-project" → true,
  "not-found" → false, "not-a-participant" → false,
  "boundary-violation" → false, "some-future-reason" → false, "" → false

…on_ref

Add TestDEF142_AC7_ConversationRef_ThroughRealMux that drives the CLI
reference path through srv.Handler().ServeHTTP() with a real agent JWT
token. The request traverses the full mux chain: guarded → agent route
→ agent action → handleAgentOutboundMessage → Resolve(conversation_ref)
→ DEF-138 auth → message dispatch.

The test verifies the stored message lands in the resolved group
conversation, not in a DM from the derivation fallback.

Mandatory mutation evidence: removing the Resolve block from the handler
causes the test to go red — the message lands in a DM (derivation) with
a different conversation_id than the expected group conversation.

Mutation output:
  expected: group conv "15020c94-..." (resolved via #d142-ac7-thread)
  actual:   DM conv "8da84c8f-..." (derivation fallback)
  Clean restore: git diff handlers_agent_messaging.go → empty

…ject_id

Send raw JSON body containing a foreign project_id in the request,
creating two candidate sources for ProjectID (body vs authenticated
agent token). The test proves the authenticated source always wins:
under a "prefer-when-present" mutation the foreign thread resolves
and DEF-138 auth rejects the project mismatch (400→403), catching
the provenance violation.

Brings tranche-g up to date with main following the merge of the
small-fix PRs (GoogleCloudPlatform#1467-GoogleCloudPlatform#1472). 41 main-only commits, 105 tranche-g-only.
Merge base 9f66cd5.

Two conflicts, both in const blocks:

1. pkg/hub/errors.go - purely additive. Ours adds 6 codes (conversation
   resolution + DEF-126 addressee), main adds 11 (B5 governance). No name
   collisions and no string-value collisions; kept both. 27 codes total,
   verified unique on both name and value.

2. pkg/store/concurrency.go - NOT additive. Both sides independently
   allocated 0x5C100014: ours to LockDataMigrations, main's to
   LockWebchatMigration. Keeping both verbatim would have shipped two
   distinct advisory locks sharing one value, silently serialising
   against each other on Postgres multi-replica. Main shipped first, so
   ours was renumbered to 0x5C100015 (next free; allocation is
   contiguous 0x5C100001-0x5C100014, plus 0x5C100020 and 0x5C1010xx).
   Advisory locks are ephemeral session locks - never persisted, no-op
   on SQLite - so renumbering is safe.

Also registers main's two new keys (LockWebchatMigration,
LockRecoveryAuthz) in the TestAdvisoryLockKeys_AllUnique registry. Main
added both without registering either; main's own
TestAdvisoryLockKeys_NonOverlapping samples 9 of 24 keys and covers
none of the three keys in play, so it passed throughout the collision.
The tranche-g registry test caught it: Check 2 (source-drift regex)
named both unregistered keys, and with the collision reintroduced as a
mutation, Check 1 reported "duplicate advisory lock key value
0x5C100014: LockWebchatMigration and LockDataMigrations".

Pre-existing on tranche-g before the main merge - the identical 14 files
fail `gofmt -l` at 8795a70, and main and tranche-g carry the same
`go 1.26.1` directive, so this is not toolchain skew and not merge
fallout.

The drift accumulated silently because `fmt-check` is a target of
`make ci` but is NOT a job in .github/workflows/ci.yml. Nothing in the
blocking gate ever ran it, so local `make ci` has been red on this
branch while CI stayed green.

Changes are comment and whitespace only. Thirteen files are pure
whitespace (const-block comment alignment). cmd/boot_m9_test.go also
gains two `//` lines from gofmt's doc-comment reflow around an indented
code block - comment text only, no code tokens. `git diff -w
--ignore-blank-lines` is empty for all other thirteen.

…merge

The main merge (b4d78c0) left pkg/hub test code uncompilable. Main's
authorization refactor (a61cddd) removed the policy concept and
renamed createProjectMembersGroupAndPolicy -> createProjectMembersGroup,
updating every call site it could see. Tranche-g had three it could not:
two in handlers_broker_inbound_test.go and def135_test.go (files both
branches edited) and one in a tranche-g-only test file.

Git merged both sides cleanly. handlers_broker_inbound_test.go ended up
with four call sites on the new name from main's rename and one on the
old name from tranche-g, in the same file, with no conflict raised.

Pure rename: identical signature
(ctx, project *store.Project, callerUserID ...string), same defining
file, and all three sites pass only (ctx, project).

Why this got through: `go build ./...` passes because only TEST code
referenced the old name, and the blocking CI gate is `make test-fast`
(-tags no_sqlite), which excludes these files. Verified now with
`go vet ./...` over the whole tree, which compiles test files in every
package and reports zero errors.

…nt (DEF-152)

The guard at line 210 in handleAgentOutboundMessage rejected any request
with an empty recipient BEFORE the conversation_ref resolver at line 334
could run. Since conv:<uuid>, #<thread>, and @<agent> references carry no
explicit recipient by design, they were always rejected with "recipient is
required."

Changes:
- Relax the guard: requests with a conversation_ref but no recipient now
  pass through to the resolver. Requests with NEITHER are still 400 with
  the original error message.
- After conversation resolution + DEF-138 authorization, derive the
  addressee from the resolved conversation's DM key for direct
  conversations. For group conversations (no single addressee), refuse
  explicitly rather than guessing.
- Security: the addressee is derived from the conversation's participant
  set (the DM key), never from request input. Non-user addressees
  (agent-to-agent DMs) are refused on this user-delivery endpoint.

Tests added:
1. conv:<uuid> with no recipient → 2xx, correct recipient derived from
   DM key (the missing test that would have caught this).
2. #<thread> with no recipient → clear refusal for group conversations.
3. Neither recipient nor conversation_ref → still 400, message unchanged.
4. Conversation the sender is not a participant of → refused, no
   disclosure of project IDs.
5. Backwards compatibility: ref + explicit recipient still works.

Two mutations survived the initial test suite:

1. Replacing the sender-matching if/else with `addrKind, addrID = kindB, idB`
   (always take side B) passed all tests because the fixture always had the
   agent on side A of the DM key.

2. Replacing `if addrKind != "user"` with `if false` passed all tests because
   no test exercised an agent-to-agent DM through this endpoint.

Added:
- TestDEF152_SenderOnSideB_DerivedAddresseeStillCorrect: uses a hand-crafted
  DM key with the user on side A and the agent on side B. Under mutation 1,
  the test fails (400 instead of 200: "non-user addressee").
- TestDEF152_NonUserAddressee_AgentToAgentDM_Refused: sends conv:<uuid>
  pointing to an agent-to-agent DM. Under mutation 2, the test fails (500
  instead of 400: user lookup fails for agent ID).

Both flags are now refused early in RunE with an actionable error
naming the replacement command (scion broadcast / scion broadcast --all).
The flags remain registered (hidden, using BoolP) so cobra parses them
without an "unknown flag" error, but they are no longer bound to
package-level variables.

Changes:
- Remove msgBroadcast/msgAll variables and all conditional branches
- Clean up sendMessageViaHub signature: remove dead broadcast/all params
- Remove local-mode broadcast dispatch (fan-out, WaitGroup, etc.)
- Remove Broadcasted field from buildStructuredMessage (only
  cmd/broadcast.go and server-side handleProjectBroadcast set it now)
- Remove unused imports (state, config)
- Reword conv:/# ref error messages to be unmistakably by-design:
  state the positive case (works inside an agent container where
  SCION_AGENT_NAME is set)
- Update tests: remove broadcast/all test functions, update call
  signatures, fix Changed state cleanup across test isolation
- Update docs (cli.md, messaging.md, SKILL.md)

Security: server-side Broadcasted=true forcing in handleProjectBroadcast
is untouched (verified by check-security-marker-gates).

scion broadcast is not in agentAllowed, so the refusal error must not
recommend it when running in agent mode. In agent mode the error now
says broadcasting is not available and tells agents to address
recipients explicitly. Human mode keeps the existing scion broadcast
pointer.

Also fixes SKILL.md (agent-facing doc) to stop recommending
scion broadcast — states plainly that broadcasting is not available
in agent mode. Human-facing docs-site pages are unchanged.

Adds tests for both agent-mode refusals asserting the error does NOT
contain "scion broadcast".

broadcast is absent from agentAllowed by construction (it is an
allowlist), but nothing in the test suite caught accidental grants.
Injecting "broadcast": true into the map left all tests green.

Add broadcast to:
- buildTestTree: so applyModeRestrictions sees and removes it
- TestApplyModeRestrictions_Agent absent list: pins end-to-end removal
- TestAgentAllowedList notAllowed list: pins the map entry directly

Mutation-tested: injecting "broadcast": true into agentAllowed causes
both TestAgentAllowedList and TestApplyModeRestrictions_Agent to fail;
reverting restores green.

Removes the --broadcast and --all flags from 'scion message'. Flags remain
registered but hidden as tombstones so they refuse with an actionable error
rather than cobra's bare 'unknown flag'. Refusal is mode-aware: agents are
told to address recipients explicitly, since 'scion broadcast' is not in the
agentAllowed allowlist and they cannot run it.

Also pins broadcast's absence from agent mode (DEF-155), which was previously
true by construction but asserted by no test.

Reviewed by cr-msg-bcast: APPROVE, no Critical or Required findings.